chore(lint): eslint react-table #3693

evwilkin · 2020-02-06T20:31:56Z

What: Towards #3592 - linting for react-table

patternfly-build · 2020-02-06T20:39:20Z

PF3 preview: https://patternfly-react-pr-3693-pf3.surge.sh
PF4 preview: https://patternfly-react-pr-3693.surge.sh
A11y report: https://patternfly-react-pr-3693-coverage.surge.sh

codecov-io · 2020-02-06T20:49:16Z

Codecov Report

❗ No coverage uploaded for pull request base (master@54a4835). Click here to learn what that means.
The diff coverage is 33.33%.

@@            Coverage Diff            @@
##             master    #3693   +/-   ##
=========================================
  Coverage          ?   71.26%           
=========================================
  Files             ?      779           
  Lines             ?    10489           
  Branches          ?     2261           
=========================================
  Hits              ?     7475           
  Misses            ?     2592           
  Partials          ?      422

Flag	Coverage Δ
#misc	`95.45% <ø> (?)`
#patternfly3	`85.89% <0%> (?)`
#patternfly4	`60.2% <100%> (?)`

Impacted Files	Coverage Δ
...ly-4/react-core/src/components/Card/CardFooter.tsx	`100% <ø> (ø)`
...nfly-4/react-core/src/components/Card/CardBody.tsx	`100% <ø> (ø)`
...act-core/src/components/DataList/DataListCheck.tsx	`30% <ø> (ø)`
...tternfly-4/react-core/src/components/Card/Card.tsx	`100% <ø> (ø)`
...3/patternfly-react/src/components/Slider/Slider.js	`89.28% <0%> (ø)`
...ct-core/src/components/DataList/DataListAction.tsx	`84.61% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54a4835...f8e4475. Read the comment docs.

packages/patternfly-4/react-table/src/components/Table/BodyCell.tsx

packages/patternfly-4/react-table/src/components/Table/SelectColumn.tsx

dlabrecq · 2020-02-07T14:10:17Z

packages/patternfly-4/react-table/src/components/Table/Body.tsx

  rowKey = 'id' as string,
-  onRowClick = (event: React.MouseEvent, row: IRow, rowProps: IExtraRowData, computedData: IComputedData) => undefined as OnRowClick,
+  /* eslint-disable @typescript-eslint/no-unused-vars */
+  onRow = (...args: any) => Object,


onRow and onRowClick appear to be used?

Both onRow and onRowClick were throwing TS errors - it looks like the unused variable errors were for the arguments to both functions (...args for onRow, and event/row/rowProps/computedData for onRowClick)

Can we track down how onRow is used and give it a proper type?

Also, for the unused variables, just put underscores before the variable names. Like _event: React.MouseEvent. Then you can remove the eslint-disable.

redallen · 2020-02-07T15:28:24Z

packages/patternfly-4/react-table/src/components/Table/Body.tsx

  rowKey = 'id' as string,
-  onRowClick = (event: React.MouseEvent, row: IRow, rowProps: IExtraRowData, computedData: IComputedData) => undefined as OnRowClick,
+  /* eslint-disable @typescript-eslint/no-unused-vars */
+  onRow = (...args: any) => Object,


Can we track down how onRow is used and give it a proper type?

Also, for the unused variables, just put underscores before the variable names. Like _event: React.MouseEvent. Then you can remove the eslint-disable.

packages/patternfly-4/react-table/src/components/Table/Body.tsx

packages/patternfly-4/react-table/src/components/Table/SelectColumn.tsx

packages/patternfly-4/react-table/src/components/Table/base/columns-are-equal.ts

packages/patternfly-4/react-table/src/components/Table/base/evaluate-formatters.ts

redallen · 2020-02-07T15:35:09Z

packages/patternfly-4/react-table/src/components/Table/base/body.tsx

 class BaseBody extends React.Component<BodyProps, {}> {
  static defaultProps = {
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
    onRow: (...args: any) => Object


A proper type for onRow would be wonderful.

packages/patternfly-4/react-table/src/components/Table/base/evaluate-transforms.ts

packages/patternfly-4/react-table/src/components/Table/base/resolve-row-key.ts

packages/patternfly-4/react-table/src/components/Table/utils/decorators/headerCol.tsx

redallen

It's fine that we can't solve the onRow prop for now. We can fix it in the future.

patternfly-build · 2020-02-10T19:01:32Z

Your changes have been released in:

@patternfly/react-catalog-view-extension@1.3.12
@patternfly/react-core@3.136.9
@patternfly/react-docs@4.18.14
@patternfly/react-inline-edit-extension@2.16.11
demo-app-ts@3.22.9
@patternfly/react-table@2.26.11
@patternfly/react-topology@2.13.11
@patternfly/react-virtualized-extension@1.3.107

Thanks for your contribution! 🎉

dlabrecq · 2020-02-12T22:22:14Z

packages/patternfly-4/react-table/src/components/Table/Table.tsx

 }

-export interface IHeaderRow extends ColumnType { }
+export type IHeaderRow = ColumnType;


Should we have changed this from interface to type?

redallen and others added 27 commits February 5, 2020 09:55

use eslint

6f20b9b

tweak config

6b04be4

tweak config

083578a

fix(docs): fixed eslintrc error

0ecfc1b

fix(docs): lint:ts --fix

2b399d5

fix(docs): eslint errors

df87c32

fix(docs): more eslint errors

d7d27be

fix(docs): fix more eslint errors

70dc57b

fix(docs): fixed last eslint error

2866acf

fix(docs): fix TS errors

ce475bd

fix(docs): fixed DataToolbar tsc error

3d0f480

fix(docs): topologycontrolbar tsc error

d394800

fix(docs): topologysidebar tsc error

1adacd1

fix(docs): updated snapshot

dbbd385

fix(docs): updated snapshot

8ecc569

fix(docs): working through gutters type errors

7fb7bad

fix(docs): fixing Gallery ts error

d796a7b

fix(lint): fixed linting errors

6a977eb

fix(eslint): committing yarn lock

0e04e69

merge yarn.lock

1c7ee5e

fix(eslint): fixed additional linting errors

2acf2ea

fix(DataListItem): fixed a11y aria-selected issue

bae18cf

fix(lint): update lint:ts for react-table

bc98e7c

fix(lint): resolved eslint errors in react-table

193481e

fix(react-tables): resolved eslint errors

54a4835

fix(react-tables): fixed merge conflicts

bd51f63

updated lint:md to target only react-core

190c1d5

dlabrecq reviewed Feb 7, 2020

View reviewed changes

evwilkin requested a review from redallen February 7, 2020 15:24

redallen suggested changes Feb 7, 2020

View reviewed changes

dlabrecq previously approved these changes Feb 10, 2020

View reviewed changes

redallen previously approved these changes Feb 10, 2020

View reviewed changes

redallen changed the title ~~Fix/eslint tables~~ chore(lint): eslint tables Feb 10, 2020

redallen changed the title ~~chore(lint): eslint tables~~ chore(lint): eslint react-table Feb 10, 2020

resolved feedback

7985b41

evwilkin dismissed stale reviews from redallen and dlabrecq via 7985b41 February 10, 2020 16:27

dlabrecq previously approved these changes Feb 10, 2020

View reviewed changes

fixed a11y error

f8e4475

evwilkin dismissed dlabrecq’s stale review via f8e4475 February 10, 2020 18:31

redallen approved these changes Feb 10, 2020

View reviewed changes

evwilkin requested a review from dlabrecq February 10, 2020 18:45

dlabrecq approved these changes Feb 10, 2020

View reviewed changes

redallen merged commit e628ae1 into patternfly:master Feb 10, 2020

dlabrecq reviewed Feb 12, 2020

View reviewed changes

evwilkin mentioned this pull request Feb 17, 2020

Enforce TS linting on new PRs #3592

Closed

chore(lint): eslint react-table #3693

chore(lint): eslint react-table #3693

Uh oh!

Conversation

evwilkin commented Feb 6, 2020

Uh oh!

patternfly-build commented Feb 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Feb 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

dlabrecq Feb 7, 2020

Choose a reason for hiding this comment

Uh oh!

evwilkin Feb 7, 2020

Choose a reason for hiding this comment

Uh oh!

redallen Feb 7, 2020

Choose a reason for hiding this comment

Uh oh!

redallen Feb 7, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

redallen Feb 7, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

redallen left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented Feb 10, 2020

Uh oh!

dlabrecq Feb 12, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

patternfly-build commented Feb 6, 2020 •

edited

Loading

codecov-io commented Feb 6, 2020 •

edited

Loading